Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add trycatch support #186

Merged
merged 30 commits into from
May 6, 2020
Merged

add trycatch support #186

merged 30 commits into from
May 6, 2020

Conversation

lightszero
Copy link
Member

@lightszero lightszero commented Jan 21, 2020

working

@@ -1348,11 +1348,24 @@ private int _ConvertNewObj(OpCode src, NeoMethod to)
}
else if (_type.DeclaringType.FullName.Contains("Exception"))
{
//对异常对象要留一个,因为catch 会处理这个栈
_Convert1by1(VM.OpCode.NOP, src, to);//空白
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the optimizer to delete NOP, does that affect this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#if DEBUG ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not worry,optimzer is safe to delete any NOP

@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 14, 2020

@lightszero dalao, vm has merged the try-catch pr, we can continue this pr now. 😂

@lightszero
Copy link
Member Author

go go go

@lightszero
Copy link
Member Author

@Tommo-L many unittest had fail, help

@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 16, 2020

@Tommo-L many unittest had fail, help

Ok, I'll help fix these ut.

@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 16, 2020

This pr blocked by #219, as neo-core have merged the offset pr, most ut need to adjust.

@ShawnYun ShawnYun mentioned this pull request Apr 26, 2020
Tommo-L
Tommo-L previously approved these changes Apr 29, 2020
Insert1(VM.OpCode.DROP, "", to);
ConvertPushString("usererror", src, to);
}
else if (pcount == 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!= and remove the empty block

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a comment for it.

@ProDog
Copy link
Contributor

ProDog commented Apr 30, 2020

Can we supports catching details of exceptions? like:

try
{
                   
}

catch (Exception ex)
{
    Runtime.Notify("error:" + ex.Message);
}

@lightszero
Copy link
Member Author

Can we supports catching details of exceptions? like:

try
{
                   
}

catch (Exception ex)
{
    Runtime.Notify("error:" + ex.Message);
}

we can do that next step.

src/Neo.Compiler.MSIL/MSIL/Converter.cs Outdated Show resolved Hide resolved
src/Neo.Compiler.MSIL/MSIL/Converter.cs Outdated Show resolved Hide resolved
@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 30, 2020

@lightszero need your help, I have tried to fix it but failed, I need more time. Could you have a look?

public static object tryWithTwoFinally()
{
	int v = 0;
	try
	{
		try
		{
			v++;
		}
		catch
		{
			v += 2;
		}
		finally
		{
			v += 3;
		}
	}
	catch
	{
		v += 4;
	}
	finally
	{
		v += 5;
	}
	return v;
}

Found by @ProDog

@lightszero
Copy link
Member Author

got it,I will check that.
but the ut had passed?so ut had problem?

@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 30, 2020

It's a new ut. The current ut all are passed. Should I send a new PR to add the new ut. And the source code is here, #186 (comment)

@lightszero
Copy link
Member Author

you could direct add ut in this pr.

@Tommo-L
Copy link
Contributor

Tommo-L commented Apr 30, 2020

Plz, merge this pr #261, I don't have the authorization to push commit directly.

@lightszero
Copy link
Member Author

@Tommo-L done,it worked.

Tommo-L
Tommo-L previously approved these changes May 2, 2020
@lightszero lightszero requested a review from shargon May 2, 2020 03:30
@Tommo-L
Copy link
Contributor

Tommo-L commented May 2, 2020

Need your last test @ProDog

@ProDog
Copy link
Contributor

ProDog commented May 6, 2020

Need your last test @ProDog

Cool, I have tested it passed.

@lightszero lightszero merged commit 9a8ccac into master May 6, 2020
@lightszero lightszero deleted the Branch_lights_trycatch branch May 6, 2020 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants